feat:support --dependency-update before upgrade #774
Conversation
Signed-off-by: kamilswiec <[email protected]>
|
@kamilswiec could you add some unit tests for this? |
|
@kamilswiec hi. I will merge this if you can add some tests for this feature. and releae new version. |
|
@kamilswiec ping. |
There was a problem hiding this comment.
Pull Request Overview
This PR adds support for the --dependency-update flag to the helm diff upgrade command, allowing automatic dependency updates before diffing. This eliminates the need to manually run helm dep up chart when chart dependencies are missing and ensures consistency with helm upgrade command arguments.
- Added
--dependency-updateboolean flag to the diff command - Implemented dependency update execution before running the diff operation
- Added helper function to execute
helm dependency updatecommand
Reviewed Changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| cmd/upgrade.go | Added dependencyUpdate flag field, command line flag registration, and logic to execute dependency update before diffing |
| cmd/helm3.go | Added updateDependencies helper function to execute helm dependency update command |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
| return err | ||
| } else { | ||
| fmt.Println(string(output)) | ||
| } |
There was a problem hiding this comment.
[nitpick] The 'else' clause is unnecessary here since the 'if' block returns early on error. Consider removing the 'else' and unindenting the output printing code for better readability.
| } | |
| } | |
| fmt.Println(string(output)) |
Copilot uses AI. Check for mistakes.
| if err != nil { | ||
| return err | ||
| } else { | ||
| fmt.Println(string(output)) |
There was a problem hiding this comment.
[nitpick] The dependency update output is printed to stdout without any context or formatting. Consider adding a prefix like 'Dependency update:' or using a logger to maintain consistency with other command output formatting in the codebase.
| fmt.Println(string(output)) | |
| log.Printf("Dependency update: %s", string(output)) |
Copilot uses AI. Check for mistakes.
When using
helm diff upgradecommand when chart dependencies inchartsare absent you have to add runhelm dep up chart.I want to be able to pass most of the arguments that I also use in
helm upgradecommand. This allows me to reduce duplication and ensurehelm diff upgradeuses the same flags ashelm upgradebe passing the same${args}variable.Tested with helm version
3.17.1.